Skip to content

Resize block buffer after allocation if significant wasted space #1674

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Jump to bottom
Merged
gh-worker-dd-mergequeue-cf854d merged 1 commit into from
Jan 20, 2026
Merged

Resize block buffer after allocation if significant wasted space #1674

gh-worker-dd-mergequeue-cf854d merged 1 commit into from
Jan 20, 2026

Conversation

@TheSafo
Copy link
Contributor

@TheSafo TheSafo commented Dec 30, 2025 •
edited
Loading

What does this PR do?

When constructing a block cache, the cache pre-allocates a buffer of max block size for each block. Payload generators then serialize bytes into that buffer. The buffer previously was never resized and thus held onto its excess memory. This made caches with small blocks hold (# blocks * max block size) in memory instead of the specified cache size.

This change resizes blocks that use under half of the pre-allocated space. This should enforce that the buffers of a cache are now no more than 2x the maximum configured cache size.

Motivation

Significant memory usage from my payload generator that has lots of smaller blocks.

Additional notes

We could probably resize every block since this is only done on lading creation anyways - wdyt? 2x was an arbitrary decision here.

@TheSafo Graphite App
Copy link
Contributor Author

TheSafo commented Dec 30, 2025 •
edited
Loading

This stack of pull requests is managed by Graphite. Learn more about stacking.

@TheSafo TheSafo changed the title resize blocks Resize blocks Dec 30, 2025
@TheSafo TheSafo changed the title Resize blocks Resize block buffer after allocation if significant wasted space Dec 30, 2025
@TheSafo TheSafo marked this pull request as ready for review December 30, 2025 15:31
@TheSafo TheSafo requested a review from a team as a code owner December 30, 2025 15:31
GeorgeHahn
GeorgeHahn approved these changes Jan 6, 2026
View reviewed changes
Copy link
Contributor

@GeorgeHahn GeorgeHahn left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This looks good to me. Thanks for investigating - this was an interesting finding.

@GeorgeHahn
Copy link
Contributor

GeorgeHahn commented Jan 7, 2026

We could probably resize every block since this is only done on lading creation anyways - wdyt? 2x was an arbitrary decision here.

I went back and forth on this, but I do like the idea of resizing all blocks. Some extra work up front to reduce memory usage at runtime is probably the right tradeoff for lading as it runs in SMP jobs. That said, I'm not too sad about the 50% worst case inefficiency either. That's about the same as we could expect from allocating the BytesMut on the fly. Your call.

This was referenced Jan 15, 2026
Merged
Merged
Open
Merged
Draft
@TheSafo TheSafo force-pushed the jsaf/resize-blocks branch from 92a7bd0 to f4e7959 Compare January 15, 2026 21:35
@TheSafo
Copy link
Contributor Author

TheSafo commented Jan 20, 2026

/merge

@gh-worker-devflow-routing-ef8351
Copy link

gh-worker-devflow-routing-ef8351 bot commented Jan 20, 2026 •
edited
Loading

View all feedbacks in Devflow UI.

2026-01-20 16:21:23 UTC ℹ️ Start processing command /merge

2026-01-20 16:22:21 UTC ℹ️ MergeQueue: pull request added to the queue

The expected merge time in main is approximately 0s (p90).

2026-01-20 16:34:48 UTC ℹ️ MergeQueue: This merge request was merged

@gh-worker-dd-mergequeue-cf854d gh-worker-dd-mergequeue-cf854d bot merged commit 6d7eb55 into main Jan 20, 2026
34 checks passed
@gh-worker-dd-mergequeue-cf854d gh-worker-dd-mergequeue-cf854d bot deleted the jsaf/resize-blocks branch January 20, 2026 16:34
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

@blt blt blt approved these changes

@GeorgeHahn GeorgeHahn GeorgeHahn approved these changes

Assignees

No one assigned

Labels

mergequeue-status: done

Projects

None yet

Milestone

No milestone

Development

Successfully merging this pull request may close these issues.

4 participants

@TheSafo @GeorgeHahn @blt